👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

When does this need to be called from a separate thread? There is only one thread in the AsyncLLM IIUC

When does this need to be called from a separate thread? There is only one thread in the AsyncLLM IIUC

Seems some nuance in our complex setup caused this and it's consistently reproducible. I've been trying to construct a small repro but not able yet

@22quinn yes this should not be required. It would only be needed if the put method was called from outside the asyncio event loop, but that should never be the case. This is the same for similar asyncio utilities such as asyncio.Queue.

On a high level, the issue is we initialize AsyncLLM from one thread, then we have a thread pool executor which dynamically call generate from different threads. I was not able to minimize the setup, but it seems a valid use case - @robertgshaw2-redhat @njhill do you think it's fine to apply this fix? I think using call_soon_threadsafe has no downside even if it's called from the same thread.
cc @xw285cornell

@22quinn I still don't think this is the right thing to do. You should definitely not be calling AsyncLLM.generate from different threads. It's an async method which should be only be called from the (single) asyncio event loop thread.

If you have code to run multiple generates in parallel, it should use parallel async tasks rather than threads. If you want to share your client code I can help to suggest how it should be changed.

There has been some discussion about supporting multi-threaded non-async use with the LLM interface (I think it "kind of" works already), which would be the correct way to go if you really want to use a threadpool.